Launcher: implement gRPC transport option handling #116
Conversation
github-actions
bot
added
documentation
greschd
force-pushed
the
feat/launcher-grpc-transport-options
branch
from
0f9f7eb to
52aa7f8
Compare
| @@ -0,0 +1,179 @@ | |||
| # Copyright (C) 2025 ANSYS, Inc. and/or its affiliates. | |||
There was a problem hiding this comment.
May be worth discussing if this should be located here, or together with the cyberchannel module.
There was a problem hiding this comment.
I like this approach to be honest. Maybe we should move it there eventually and adapt the cyberchannel function calls to use these options directly.
There was a problem hiding this comment.
Let's open an issue and start discussing it.
| Parameters | ||
| ---------- | ||
| timeout : float, default: None | ||
| timeout : |
There was a problem hiding this comment.
Here (and elsewhere) I incidentally reverted the edit to add the type. However, I'm unsure if we need to duplicate the content from the type hint: can we inject it at doc build time instead?
There was a problem hiding this comment.
Fine by me - I think if there is a typehint and the Sphinx can infer it from there, it's okay. I'm worried about what LSPs in IDEs like VSC do. I don't know if when hovering/autocompleting, the docstring would show the proper type
There was a problem hiding this comment.
The doc rendering worked correctly in the ansys-tools-product-launcher repo (see here), but not with the AutoAPI setup used here (I tried following https://sphinx-autoapi.readthedocs.io/en/latest/how_to.html).
Do you happen to know a working setup for AutoAPI + Ansys Sphinx Theme + type hints rendered in the description?
Also (side note), the doc build is complaining about duplicate descriptions due to the reimport in the abstractions module; not sure exactly why that is (I didn't investigate further), but that should probably also be addressed.
RobPasMue
left a comment
RobPasMue
left a comment
There was a problem hiding this comment.
It's looking good - I guess this will require a new minor right?
| @@ -0,0 +1,179 @@ | |||
| # Copyright (C) 2025 ANSYS, Inc. and/or its affiliates. | |||
There was a problem hiding this comment.
I like this approach to be honest. Maybe we should move it there eventually and adapt the cyberchannel function calls to use these options directly.
| @@ -0,0 +1,179 @@ | |||
| # Copyright (C) 2025 ANSYS, Inc. and/or its affiliates. | |||
There was a problem hiding this comment.
Let's open an issue and start discussing it.
| Parameters | ||
| ---------- | ||
| timeout : float, default: None | ||
| timeout : |
There was a problem hiding this comment.
Fine by me - I think if there is a typehint and the Sphinx can infer it from there, it's okay. I'm worried about what LSPs in IDEs like VSC do. I don't know if when hovering/autocompleting, the docstring would show the proper type
Yes. I'll want to actually try using it in PyACP (currently uses the old module still) before we release, though. |
greschd
force-pushed
the
feat/launcher-grpc-transport-options
branch
from
42d01f7 to
2f8e4c0
Compare
|
|
||
| LAUNCHER_ENTRY_POINT = "ansys.tools.local_product_launcher.launcher" | ||
| LAUNCHER_ENTRY_POINT = "ansys.tools.common.launcher" | ||
| DEPRECATED_LAUNCHER_ENTRY_POINT = "ansys.tools.local_product_launcher.launcher" |
There was a problem hiding this comment.
Currently, the ansys-launcher command may refer to either the version from ansys-tools-common, or the one from ansys-tools-local-product-launcher, depending on which package was installed last.
That's unavoidable since we want to reuse the ansys-launcher name, but we can ease the transition by initially accepting both entry points. Then we can upgrade with the following steps (in relatively short succession):
- initial state: plugins use the old entry point
- deprecate
ansys-tools-local-product-launcher:- it only re-exports the objects from
ansys-tools-common - remove its script entrypoint
- it only re-exports the objects from
- plugins switch to the new entry point
- (some time later) remove the
DEPRECATED_LAUNCHER_ENTRY_POINThandling here
|
@RobPasMue what's the rationale behind creating our own exception hierarchy (specifically, the use of As such, I'm trying to figure out if PyACP should go the same route, or spend extra effort to hide this change. |
Implement gRPC transport option handling for the product launcher:
with a method to instantiate the corresponding gRPC channel
an entry in the newly added
transport_optionsproperty, instead of providingan entry in the
urlsproperty.This change is needed since the URL itself is not sufficient to create a gRPC
channel.
Other changes:
ansys.tools.common.launcher